-
-
Notifications
You must be signed in to change notification settings - Fork 504
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(lint/useExhaustiveDependencies): support Preact #2105
Conversation
✅ Deploy Preview for biomejs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
CodSpeed Performance ReportMerging #2105 will not alter performanceComparing Summary
|
Let's hold off the merge of this PR. Let's fix more bugs, ship some more patches, and then merge it :) |
You mean you want to do more 1.6.x releases before merging it, right? I’m having trouble saying if it’s tongue-in-cheek 😅 |
Yes. If we merge a minor (because it is), we're forced to ship a 1.7.0 |
### Linter | ||
### Parser |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did this change?
@@ -359,10 +361,6 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b | |||
|
|||
#### Bug fixes | |||
|
|||
- JavaScript lexer is now able to lex regular expression literals with escaped non-ascii chars ([#1941](https://github.com/biomejs/biome/issues/1941)). | |||
|
|||
Contributed by @Sec-ant |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this removed?
Bad merge conflict resolution?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, that was indeed a wrongly merged conflict. I'll fix right away, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary
This implements support for hooks imported from Preact instead of only React.
I have also checked two other rules:
useHookAtTopLevel
uses the naming convention to determine what is a hook, so it should already have been compatible with Preact.noRenderReturnValue
checks that the return value of ReactDOM'srender()
function isn't being used. According to Preact's documentation, theirrender()
function already doesn't return anything, so I think that hook doesn't apply to Preact.I hope this covers all places where React/Preact compatibility might be relevant for us.
Fixes #2043.
Test Plan
Test case added.